Skip to content

Promote the File Table Reading to Main - #626

Merged
ptr727 merged 1 commit into
mainfrom
develop
Aug 8, 2026
Merged

Promote the File Table Reading to Main#626
ptr727 merged 1 commit into
mainfrom
develop

Conversation

@ptr727

@ptr727 ptr727 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

One commit, a706ddb from #625, promoting the answer to #623 to main.

What is promoted

scripts/pr_review.py reads the Copilot review's own | File | Description | table and reports it beside a partial-coverage verdict, and the docs drop a claim that was false.

  • The removed claim. "The reviewer names no file list in these rounds, so which file went unread cannot be read from the API" was wrong. Measured over 348 review bodies on the newest 120 pull requests here and 121 on Blog, 91 carry a table and every table row in either corpus belongs to one.
  • The table decides nothing, and the measurement is why. It names the whole changed set on partial and fully covered rounds alike, including all seven partials on Blog, so it is identical under both outcomes. One round here states 33 of 33 and names 32, another states 61 of 62 and names 50, and a third names GOVENANCE.md, a path no diff carries.
  • The one arm that locates a file. Promote the prose sweeps and the suppressed-findings digest to main #479 states 16 of 17 and names 16, omitting GOVERNANCE.md, and that is the only evidence on record that the unread file is real rather than an artifact of counting. status names an omitted file only in that shape, where the table is short by exactly what the counts leave unread and names nothing outside the diff.
  • Head-scoped like the counts. Three of the four partials here carry their table on the round before a push, describing the diff that push replaced, so those report as no table rather than as a stale list of unreviewed files.

Exit 42 is unchanged in every arm, and the merge decision stays the maintainer's.

State

#623 is already closed by hand, since the keyword sat on a pull request into develop. Review on #625 was one round, coverage=full shapes=ok threads=0 unresolved=0 suppressed=0 checks=6/6, and the body was read rather than the digest trusted.

Merge as a merge commit, never a squash, so develop and main stay level.

Generated with Claude Code

)

Closes #623.

**Disposition** `Amends "Find out which file a partial round skips, and
why re-requesting never clears it."` in the "A Programmatic Reading of a
Copilot Review" cluster. #623 was filed after this repository's last
`TODO.md` edit and appeared nowhere in the file, so selection step 1 was
false until this change.

## What was wrong

The `COVERAGE_IS_PARTIAL` caveat, `scripts/README.md` and the runbook
all said the reviewer names no file list in these rounds, so which file
went unread cannot be read from the API. Measured over 348 Copilot
review bodies on the newest 120 pull requests here and 121 on Blog, that
is false. 91 bodies carry a `| File | Description |` table, and every
table row in the corpus belongs to one of those tables.

## What the measurement says the table is worth

The issue proposed reading a table that names every changed file as
evidence that the count is a reporting artifact. It cannot carry that,
and the corpus is what says so.

| round | states | names | reading |
| --- | --- | --- | --- |
| #476 | 10 of 11 | all 11 | table contradicts the count |
| #592 | 2 of 3 | all 3 | table contradicts the count |
| #479 | 16 of 17 | 16, omitting `GOVERNANCE.md` | table corroborates
the count and names the file |
| #609 | 61 of 62 | 50 of 62 | short by 12 where the count is short by 1
|
| #574 | 33 of 33, full | 32, omitting `TODO.md` | a short table on a
fully covered round |
| #606 | 7 of 7 | names `GOVENANCE.md` | a path no diff carries |

On Blog every table names exactly the changed set, on all seven partial
rounds and on every full round alike. A reading identical under both
outcomes discriminates neither, so a full table is reported as
corroborating nothing rather than as a miscount. #574 kills the
converse, since a short table sits on a round that read everything.

## What ships

- `Q_FULL` selects the pull request's own `files`, windowed at 100 with
`hasNextPage` carried, since a path outside a short window reads exactly
like a path the reviewer left out and the record holds a pull request of
301 changed files.
- `file_table` reads the table, quotations dropped for the reason the
coverage line's are.
- `head_table` takes it from any round covering the current head and
never from a round before a push. Three of the four partials here carry
their table on the pre-push round, describing a diff that push replaced,
and comparing that against the current changed files would name a file
unreviewed on a stale list. Thirteen commits here carry more than one
round, and on one of them a round with a table sits beside a round
without, so which of the two the verdict reads must not decide whether a
table is found.
- `table_against_diff` prints one sentence and decides nothing. It names
an omitted file only where the table is short by exactly what the counts
leave unread **and** names nothing outside the diff, that second guard
existing because #606's typo would otherwise push the real
`GOVERNANCE.md` into the omissions and report it as the file nobody
reviewed.
- Exit `42` is unchanged in every arm, and the caveat keeps its
maintainer-decision framing, which is the issue's third suggestion.

## Verified against live data

#479 the reviewer's own file table omits exactly the 1 file the counts
leave unread, naming GOVERNANCE.md
Blog #60 the reviewer's own file table names all 6 changed files, ... so
it corroborates nothing
    #476/#592/#609   no round covering this head carries a file table

## Gates

228 `test_pr_review` cases, plus `test_prose_lint`, `test_repo_gate`,
`spec/audit.py --selftest`, `spec/validate.py`, both `prose_lint.py`
invocations, `markdownlint-cli2` and `editorconfig-checker`, all green
locally.

Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 8, 2026 17:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Promotes the scripts/pr_review.py enhancement that reads Copilot's per-file overview table (when present) and reports what it implies (without changing the coverage verdict), and updates the runbook/docs to drop the incorrect “no file list” claim.

Changes:

  • Extend scripts/pr_review.py’s GraphQL query to fetch the PR changed-file list (windowed) and add logic to parse the review body’s | File | Description | table.
  • Print a “table vs diff” explanatory line alongside COVERAGE_IS_PARTIAL in both status output and the digest, while keeping exit code semantics unchanged.
  • Update documentation/runbook text (and TODO tracking) to reflect the measured behavior of the file table and its limited evidentiary value.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
TODO.md Updates the research record to reflect the corrected finding about the reviewer’s file table and links issue #623.
scripts/test_pr_review.py Adds fixtures and a focused test suite covering file-table parsing and “table vs diff” reporting behavior.
scripts/README.md Updates the documentation to replace the incorrect “never recoverable” claim with the measured “usually” framing and the new reporting behavior.
scripts/pr_review.py Adds files to the GraphQL payload and implements file_table + table_against_diff reporting under partial coverage.
.github/copilot-instructions.md Updates the runbook guidance to reflect the measured limitations/edge-cases of the file table in partial rounds.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ptr727
ptr727 merged commit 2fb1411 into main Aug 8, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants